feat(i18n): add Traditional Chinese locale - #3853
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found blocking i18n issues.
[P2] zh-TW locale mishandled — three places
e2e-fixture.ts:119-123lowercases locale then compares tozh-CN/zh-TW→ both become null.runtime-host-boot.ts:1629-1648passeszh-CNforzh-TWsystems.- Multiple UI fallbacks use
locale!=='en'to pick zh-CN strings, sending Simplified to Traditional users.
Fix: preserve zh-TW throughout and fix comparisons/fallbacks.
简体中文
繁中语言支持被错误归入简中。Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
bf8fe16 to
4b56a8c
Compare
4b56a8c to
4b2ce5e
Compare
4b2ce5e to
5b8f9db
Compare
|
Addressed the blocking i18n findings in
Local verification now passes: build, lint, format, full typecheck, Core (685), UI (231), Desktop locale-focused suites, and CLI locale suites. Real macOS before/after screenshots are still pending as noted in the PR body; the screenshot workflow is included in |
5b8f9db to
01d9dec
Compare
|
Hi @AcerYue0 — first off, huge thanks for taking on the Traditional Chinese locale! This is the first real proof that #2672's typed Just a quick, totally non-blocking thought since you're still in For example, the shape we just sketched for Korean might also fit here and keep each review under ~30 files:
No pressure at all to rework — you've already addressed the blocking i18n findings in On that note — we just opened #3974 + six
Totally your call — just wanted to coordinate early so we don't step on each other's Thanks again for the awesome work here 🙏 |
|
Thanks for the suggestion — I agree that the core-first split has real coordination value here, especially because the Korean work can reuse the locale contract once it lands. I plan to restructure this as two stacked slices:
I will keep the second slice based on the core slice, remove duplicated core changes from this PR, and rebase it onto I will first resolve the current upstream story conflict as part of the rebase, then post the new core PR link and updated dependency relationship here. |
|
Follow-up after checking the current type contract: I will preserve the type-safe, fully localized implementation and avoid a non-green or placeholder core PR. The safe delivery shape is to keep this as one mergeable feature PR with the core changes clearly separated in commit history; if maintainers prefer a stacked split despite that coupling, I can instead prepare a core-plus-catalog-shape slice and keep the localized surfaces as the follow-up. |
|
@me2seeks Which direction should the shared contract take upstream?
This affects |
|
Hi @AcerYue0 — thanks so much for laying this out so clearly, and for the two thoughtful follow-ups! We've talked it through and here's what we'd love to go with: Contract: let's go with B — Delivery: let's keep your single mergeable PR with the core history clearly separated — no need to truly split. You nailed the constraint: That also makes the parallel work super simple: we'll park the Korean Thanks again for the amazing work and for catching the contract question early — really appreciate it! |
328e5f5 to
01a60e9
Compare
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
Generated-by: ChatGPT
f128935 to
1342876
Compare
|
@me2seeks Update: I reorganized the commit history as discussed while keeping this as one mergeable PR.
The PR head is now |
|
This is a synthesis of five independent blind reviews at exact head What I checked myself:
Findings per lane (file:line anchored): #385 Core locale contract & legacy
#386 Desktop locale authority & persistence boundary — CODE NO-GO, 1×P2 (Luna-Deep)
#387 zh-TW copy & fallback coverage — GO (conditional), 2×P3 (DSv4F)
#388 E2E fixture & screenshot workflow — GO (Muse-Reviewer)
#389 Feature necessity & convergence — FEATURE NECESSITY GO / LARGE-DIFF RETAIN / GLOBAL-DESIGN CONDITIONAL GO, not merge-ready (Feature)
What I did not judge:
Gate: Standards 1×P2 (authority crash, #386) + Spec CONDITIONAL GO (four #389 blockers) + 3×P3 copy fixes remain; hosted
|
Summary
Add separate
zh-CNandzh-TWresolved locales, including Taiwan-localized UI catalogs and system-locale aliases, while preserving the former persistedzhpreference aszh-CN.Replace the Desktop Settings language segmented control with the existing
@maka/uiSelector, offering Follow system, Simplified Chinese, Traditional Chinese, and English.Verification
npm run build— passednpm run lint— passednpm run format:check— passednpm run typecheck— passednpx knip --workspace apps/desktop— passednpx knip --workspace packages/ui— passednpm --workspace @maka/core test— 685 passednpm --workspace @maka/ui test— 231 passedTraditional Chinese locale screenshots) — added; runs onmacos-15and uploads real Settings screenshots as an artifactReview follow-up
zh-CN/zh-TWE2E fixture flags.zh-TWsurfaces, including Health Center and GitHub Copilot failures.Screenshots
This PR remains a draft until the required real UI screenshots are attached.
AI use
Select exactly one:
Tool(s) and scope: ChatGPT implemented the locale contract, Traditional Chinese catalogs, Settings selector change, review fixes, and related tests.
Checklist
Does this PR entail a change in behavior?